Align to latest Nextflow version and bump to groovy 4.0 #91
Conversation
tillenglert
force-pushed
the
Align_to_latest_nextflow
branch
2 times, most recently
from
cb1518a
to
ad09eb2
Compare
| @@ -182,7 +182,7 @@ class CO2FootprintFactory implements TraceObserverFactory { | |||
| * Factors of memory power usage | |||
| */ | |||
| // nm: size of memory available [GB] -> requested memory | |||
| Long memory = trace.get('memory') as Long | |||
There was a problem hiding this comment.
Why did you change this? I think Long would be needed here
There was a problem hiding this comment.
The function returns a List<Double>, and within this list the memory. So to return it actually as said list the memory needs to be cast to double. If the memory is returned as memory, the function would need to be set to return something like List<numerical> which in turn would need casting to doubles again. So in this case I would say the lesser evil is to cast the memory to double, even though it is a non float number.
There was a problem hiding this comment.
ok, I don't get the exact problem. Here it's also accessed like this: https://github.com/nextflow-io/nextflow/blob/949181053afce9e66ebde1ccff3c725fb2f61697/modules/nextflow/src/main/groovy/nextflow/trace/ReportSummary.groovy#L52
There was a problem hiding this comment.
(it was about this list:
)There was a problem hiding this comment.
conclusion: although long would be more optimal, using double should be sufficient to represent the memory values here
There was a problem hiding this comment.
Maybe we can convert the returned list to a map in a follow-up PR
There was a problem hiding this comment.
I added a comment and a todo to the cast, which is now later to calculate the nm and respective used energy with the full precision memory
Signed-off-by: Till Englert <[email protected]>
Signed-off-by: Till Englert <[email protected]>
Signed-off-by: Till Englert <[email protected]>
Signed-off-by: Till Englert <[email protected]>
Signed-off-by: Till Englert <[email protected]>
Signed-off-by: Till Englert <[email protected]>
…full precision data. Also add a comment, why it is casted and add TODO Signed-off-by: Till Englert <[email protected]>
tillenglert
force-pushed
the
Align_to_latest_nextflow
branch
from
fb4f798
to
96db2e7
Compare
|
The force push was to sign off every commit using, which is now required. |
This PR will align the plugin to the latest nextflow version (24.08.0-edge).
It will deprecate all nextflow versions prior to 24.01.0-edge.
(I might need to add some updates to the test workflows, but we'll see if that can be changed apart of this PR)